-
Notifications
You must be signed in to change notification settings - Fork 292
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add an assertion to HashSet::get_or_insert_with
#518
Conversation
Since the Rust libs-api team considers it problematic for `HashSet<T>` to be unreliable with well-behaved `T` (not user-controlled), we should not allow mismatches to be inserted through `get_or_insert_with`.
I didn't notice #400 before, but I think that's doing a lot more than is really needed for the immediate problem. For instance, I don't think it's important to compare the hashes, because that calculation is not under user control here. And value equality already implies hash equality, especially for known-good non-user types. |
I can correct the code if necessary. |
I removed unnecessary code. Now the comparison is carried out by values. I don’t think this will affect performance. I use the |
For some background, this came up while I was trying to remove |
Well, But You need to either rewrite |
That would be unsound, because a user could follow that with a We could add a similar |
That's right, I forgot about the |
Subsumed by #555. |
Since the Rust libs-api team considers it problematic for
HashSet<T>
to be unreliable with well-behaved
T
(not user-controlled), we shouldnot allow mismatches to be inserted through
get_or_insert_with
.